bug: Improved the feedback form UI issue #61 #75
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
@vishnuprasad2004 is attempting to deploy a commit to the bunty's projects Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe changes involve modifications to the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Thank you for submitting your pull request! 🙌 We'll review it as soon as possible. In the meantime, please ensure that your changes align with our CONTRIBUTING.md. If there are any specific instructions or feedback regarding your PR, we'll provide them here. Thanks again for your contribution! 😊 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (3)
frontend/src/App.css (3)
21-23: LGTM: Well-structured rating button styles.The
.rating-buttonclass effectively uses Tailwind CSS utility classes to create an appealing and interactive button style. This aligns well with the PR objective of improving the feedback form UI.Consider adding a hover state to enhance interactivity:
.rating-button { - @apply flex-grow bg-neutral-50 text-center p-2 rounded-md cursor-pointer select-none + @apply flex-grow bg-neutral-50 text-center p-2 rounded-md cursor-pointer select-none hover:bg-neutral-100 transition-colors }
25-27: LGTM: Appropriate styling for custom radio inputs.Hiding the default radio inputs allows for custom styling of the rating stars, which aligns with the PR objective.
To improve accessibility, consider adding a visually hidden label for screen readers:
.visually-hidden { position: absolute; width: 1px; height: 1px; padding: 0; margin: -1px; overflow: hidden; clip: rect(0, 0, 0, 0); white-space: nowrap; border-width: 0; }Then, use this class on labels associated with your radio inputs.
29-32: LGTM: Well-defined star icon styles.The
.starclass effectively styles the star icons for the rating system, making them large, clickable, and visually appealing. This directly addresses the PR objective of adding a star rating field.For consistency with the
.rating-buttonclass, consider using Tailwind CSS utility classes here as well:.star { - cursor: pointer; - font-size: 2rem; - margin-bottom: 0; + @apply cursor-pointer text-4xl mb-0; }This change maintains the same styling while aligning with the project's use of Tailwind CSS.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- frontend/src/App.css (1 hunks)
- frontend/src/components/ui/FeedbackForm.jsx (4 hunks)
🔇 Additional comments (3)
frontend/src/App.css (1)
19-19: LGTM: Background color change enhances UI.The new background color (#F1EADC) provides a warm, inviting atmosphere that should improve the overall user experience of the feedback form.
frontend/src/components/ui/FeedbackForm.jsx (2)
22-25: State variables for rating feature are well initializedThe introduction of state variables
rating,hover, andtotalStarscorrectly sets up the component for handling the rating functionality.
120-145: Star rating feature is effectively implementedThe star rating system is well-implemented, utilizing state management and event handlers to provide an interactive and responsive user experience.
|
|
||
| const handleSubmit = (e) => { | ||
| e.preventDefault(); | ||
| console.log(`Name: ${name}, Email: ${email}, Feedback: ${feedback}`); | ||
| console.log(`Name: ${name}, Email: ${email}, Feedback: ${feedback}, rating: ${rating}`); |
There was a problem hiding this comment.
Avoid logging sensitive user information to the console
Logging personal information such as names, emails, and feedback to the console can lead to security and privacy concerns, especially if the console output is exposed in production environments. Consider removing the console.log statement or ensure it's only used during development.
| {/* <label | ||
| htmlFor="name" | ||
| className="block text-sm font-medium text-[#004D43]" | ||
| className="block text-sm font-medium text-white" | ||
| > | ||
| Name | ||
| </label> | ||
| </label> */} |
There was a problem hiding this comment.
Restore labels for form inputs to improve accessibility
Commenting out the labels for the Name, Email, and Feedback inputs can negatively impact accessibility. Screen readers rely on labels to convey the purpose of form controls to users with visual impairments. Placeholders are not a substitute for labels. Please restore the labels or use aria-label or aria-labelledby attributes to ensure the form is accessible to all users.
Also applies to: 87-92, 104-109
|
@RamakrushnaBiswal What do you think ?? |
|
@vishnuprasad2004 not alligned properly |
The space under the submit button is removed since that space is being wasted when we don't get notified. The notification still comes. |
|
@vishnuprasad2004 conflicts |
|
Aligned and centered, just added a tailwind class :) |
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- frontend/src/components/ui/FeedbackForm.jsx (4 hunks)
🧰 Additional context used
🔇 Additional comments (1)
frontend/src/components/ui/FeedbackForm.jsx (1)
29-29: Avoid logging sensitive user information to the consoleLogging personal information like names, emails, feedback, and ratings can lead to security and privacy concerns, especially in production environments. Consider removing the
console.logstatement or ensure it's only used during development.
| const [rating, setRating] = useState(null); | ||
|
|
||
| const [hover, setHover] = useState(null); | ||
| const [totalStars, setTotalStars] = useState(5); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Simplify totalStars to a constant
Since totalStars represents a fixed value and setTotalStars is not used, you can replace the state variable with a constant to reduce unnecessary state management.
Apply this diff to simplify:
-const [totalStars, setTotalStars] = useState(5);
+const totalStars = 5;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const [totalStars, setTotalStars] = useState(5); | |
| const totalStars = 5; |
| setSubmitted(true); | ||
| setTimeout(() => { | ||
| setName(""); | ||
| setEmail(""); | ||
| setFeedback(""); | ||
| setRating(null); |
There was a problem hiding this comment.
Reset the hover state after form submission
When resetting the form fields after submission, consider also resetting the hover state to prevent any lingering visual effects on the stars.
Apply this change to reset the hover state:
setName("");
setEmail("");
setFeedback("");
setRating(null);
+setHover(null);
setSubmitted(false);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| setRating(null); | |
| setName(""); | |
| setEmail(""); | |
| setFeedback(""); | |
| setRating(null); | |
| setHover(null); | |
| setSubmitted(false); |
|
@vishnuprasad2004 conflicts :( |
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 14023932 | Triggered | MongoDB Credentials | f4c60cb | backend/.env | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secret safely. Learn here the best practices.
- Revoke and rotate this secret.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
|
Merge conflict resolved Idk why a MongoDB credentials error is coming in my PR, my work was only in the frontend |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (5)
frontend/src/App.css (3)
21-23: LGTM: Well-structured rating button styles.The
.rating-buttonclass is well-defined using Tailwind CSS utility classes. It provides a good set of styles for a rating button.Consider adding a hover state to improve user interaction:
.rating-button { - @apply flex-grow bg-neutral-50 text-center p-2 rounded-md cursor-pointer select-none + @apply flex-grow bg-neutral-50 text-center p-2 rounded-md cursor-pointer select-none hover:bg-neutral-100 }
25-27: LGTM: Radio inputs hidden for custom styling.Hiding radio inputs is a good approach for creating custom-styled radio buttons.
To improve accessibility, consider using
opacity: 0instead ofdisplay: none. This allows the input to remain focusable:input[type="radio"] { - display: none; + opacity: 0; + position: absolute; }
29-33: LGTM: Appropriate styles for star rating elements.The
.starclass provides good styling for star rating elements with an appropriate cursor and font size.The
margin-bottom: 0might be unnecessary if there's no default margin. Consider removing it unless it's addressing a specific layout issue:.star { cursor: pointer; font-size: 2rem; - margin-bottom: 0; }🧰 Tools
🪛 Biome
[error] 33-33: expected
}but instead the file endsthe file ends here
(parse)
README.md (2)
135-136: Replace hard tabs with spaces for consistent rendering.Markdownlint has detected hard tabs on lines 135 and 136. To ensure consistent rendering across different platforms and editors, it's recommended to use spaces for indentation in Markdown files.
Replace the hard tabs with spaces. Most editors can be configured to automatically convert tabs to spaces.
🧰 Tools
🪛 Markdownlint
135-135: Column: 1
Hard tabs(MD010, no-hard-tabs)
136-136: Column: 1
Hard tabs(MD010, no-hard-tabs)
Line range hint
1-137: Enhance README clarity and completeness.The README is well-structured and informative. Consider the following suggestions to further improve its effectiveness:
- Add a table of contents for easier navigation.
- Include information about the project's current status or roadmap.
- Provide more detailed instructions in the "Getting Started" section, including any environment setup requirements.
- Consider adding a "Frequently Asked Questions" section to address common queries.
- Update the "Mentors" section with actual mentor information once available.
These enhancements will make the README more comprehensive and user-friendly for potential contributors and users.
🧰 Tools
🪛 Markdownlint
135-135: Column: 1
Hard tabs(MD010, no-hard-tabs)
136-136: Column: 1
Hard tabs(MD010, no-hard-tabs)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- README.md (1 hunks)
- frontend/src/App.css (1 hunks)
- frontend/src/components/ui/FeedbackForm.jsx (5 hunks)
🧰 Additional context used
🪛 Markdownlint
README.md
135-135: Column: 1
Hard tabs(MD010, no-hard-tabs)
136-136: Column: 1
Hard tabs(MD010, no-hard-tabs)
🪛 Biome
frontend/src/App.css
[error] 33-33: expected
}but instead the file endsthe file ends here
(parse)
🔇 Additional comments (6)
frontend/src/App.css (2)
21-21: LGTM: Appropriate background color added.The light beige background color (#F1EADC) is a good choice for a cafe-themed application, providing a warm and inviting feel.
33-33: Ignore static analysis error: No missing closing brace.The static analysis tool reports a missing closing brace, but this appears to be a false positive. The file structure is correct, and all braces are properly closed. No action is needed.
🧰 Tools
🪛 Biome
[error] 33-33: expected
}but instead the file endsthe file ends here
(parse)
frontend/src/components/ui/FeedbackForm.jsx (3)
5-5: LGTM: Import statement for FaStarThe import of
FaStarfromreact-icons/fa6is correctly placed and will be used for the new star rating feature.
188-188: LGTM: Improved submit button stylingThe updated styling for the submit button enhances its appearance and provides a clear hover effect. The changes are consistent with the form's overall design and improve user interaction.
Line range hint
1-224: Summary of FeedbackForm.jsx reviewOverall, the changes to the FeedbackForm component enhance its functionality and user experience. The addition of the star rating feature is a valuable improvement. However, there are several areas that require attention:
- Optimize the
totalStarsstate to a constant.- Address the security concern with logging sensitive information.
- Improve form accessibility by restoring labels or using aria attributes.
- Enhance the accessibility and usability of the star rating component.
- Refine the animation of the feedback confirmation message.
Addressing these points will significantly improve the component's quality, accessibility, and performance. Great work on the enhancements, and I look forward to seeing the refinements!
README.md (1)
135-141: LGTM: Contributors list updated correctly.The addition of Dev Mishra to the contributors list has been implemented correctly, maintaining consistency with the existing format.
🧰 Tools
🪛 Markdownlint
135-135: Column: 1
Hard tabs(MD010, no-hard-tabs)
136-136: Column: 1
Hard tabs(MD010, no-hard-tabs)
| const [rating, setRating] = useState(null); | ||
|
|
||
| const [hover, setHover] = useState(null); | ||
| const [totalStars, setTotalStars] = useState(5); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider optimizing totalStars state
The new state variables for rating and hover are correctly implemented. However, totalStars could be optimized:
const [rating, setRating] = useState(null);
const [hover, setHover] = useState(null);
const totalStars = 5; // Changed from useState to a constantThis optimization is beneficial because totalStars appears to be a fixed value that doesn't change during the component's lifecycle. Using a constant instead of a state variable can slightly improve performance and readability.
| console.log(`Name: ${name}, Email: ${email}, Feedback: ${feedback}, rating: ${rating}`); | ||
| setSubmitted(true); | ||
| setTimeout(() => { | ||
| setName(""); | ||
| setEmail(""); | ||
| setFeedback(""); | ||
| setRating(null); | ||
| setSubmitted(false); | ||
| }, 3000); |
There was a problem hiding this comment.
Security concern and incomplete state reset
- Remove or modify the
console.logstatement to avoid logging sensitive user information:
console.log(`Feedback submitted with rating: ${rating}`);- Reset the
hoverstate along with other form fields:
setRating(null);
setHover(null); // Add this lineLogging sensitive information can lead to security vulnerabilities if logs are exposed. Additionally, resetting the hover state ensures a clean slate for the next submission.
| <div className="bg-[#004D43] rounded-xl p-3 pt-4 h-fit"> | ||
| <form onSubmit={handleSubmit} className="space-y-4"> | ||
| <div> | ||
| <label | ||
| {/* <label | ||
| htmlFor="name" | ||
| className="block text-sm font-medium text-[#004D43]" | ||
| className="block text-sm font-medium text-white" | ||
| > | ||
| Name | ||
| </label> | ||
| </label> */} | ||
| <input | ||
| type="text" | ||
| id="name" | ||
| value={name} | ||
| placeholder="Name" | ||
| onChange={(e) => setName(e.target.value)} | ||
| required | ||
| className="mt-1 block w-full border border-gray-300 rounded-md shadow-sm py-2 px-3 focus:outline-none focus:ring-[#004D43] focus:border-[#004D43]" | ||
| /> | ||
| </div> | ||
| <div> | ||
| <label | ||
| {/* <label | ||
| htmlFor="email" | ||
| className="block text-sm font-medium text-[#004D43]" | ||
| > | ||
| </label> | ||
| </label> */} | ||
| <input | ||
| type="email" | ||
| id="email" | ||
| placeholder="Email" | ||
| value={email} | ||
| onChange={(e) => setEmail(e.target.value)} | ||
| required | ||
| className="mt-1 block w-full border border-gray-300 rounded-md shadow-sm py-2 px-3 focus:outline-none focus:ring-[#004D43] focus:border-[#004D43]" | ||
| /> | ||
| </div> | ||
| <div> | ||
| <label | ||
| {/* <label | ||
| htmlFor="feedback" | ||
| className="block text-sm font-medium text-[#004D43]" | ||
| > | ||
| Feedback | ||
| </label> | ||
| </label> */} | ||
| <textarea | ||
| id="feedback" | ||
| placeholder="Your valuable feedback here" | ||
| rows="4" | ||
| value={feedback} | ||
| onChange={(e) => setFeedback(e.target.value)} | ||
| required | ||
| className="mt-1 block w-full border border-gray-300 rounded-md shadow-sm py-2 px-3 focus:outline-none focus:ring-[#004D43] focus:border-[#004D43]" | ||
| className="mt-1 block w-full border border-gray-300 rounded-md shadow-sm py-2 px-3 focus:outline-none focus:ring-[#004D43] focus:border-[#004D43] resize-none" |
There was a problem hiding this comment.
Improve form accessibility
While the styling changes and added placeholders enhance the form's appearance, commenting out the labels (lines 109-114, 126-131, 143-148) negatively impacts accessibility. Consider these options:
- Restore the labels for better accessibility:
<label htmlFor="name" className="sr-only">Name</label>
<input
type="text"
id="name"
value={name}
placeholder="Name"
// ... other props
/>- Use
aria-labelattributes if you prefer not to use visible labels:
<input
type="text"
id="name"
value={name}
placeholder="Name"
aria-label="Name"
// ... other props
/>These changes will ensure that screen readers can properly convey the purpose of each form field to users with visual impairments.
| <div className="flex flex-row justify-center gap-2"> | ||
| {[...Array(totalStars)].map((star, index) => { | ||
| const currentRating = index + 1; | ||
| return ( | ||
| <label key={index}> | ||
| <input | ||
| type="radio" | ||
| name="rating" | ||
| value={currentRating} | ||
| onChange={() => setRating(currentRating)} | ||
| /> | ||
| <span | ||
| className="star" | ||
| style={{ | ||
| color: | ||
| currentRating <= (hover || rating) ? "#ffc107" : "#e4e5e9" | ||
| }} | ||
| onMouseEnter={() => setHover(currentRating)} | ||
| onMouseLeave={() => setHover(null)} | ||
| > | ||
| <FaStar /> | ||
| </span> | ||
| </label> | ||
| ); | ||
| })} | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance accessibility and usability of star rating component
The star rating component is a good addition, but it needs improvements for accessibility and usability:
- Add
aria-labelto provide context for screen readers:
<input
type="radio"
name="rating"
value={currentRating}
onChange={() => setRating(currentRating)}
aria-label={`Rate ${currentRating} out of ${totalStars} stars`}
/>- Make the stars focusable and selectable via keyboard:
<span
className="star"
style={{
color: currentRating <= (hover || rating) ? "#ffc107" : "#e4e5e9"
}}
onMouseEnter={() => setHover(currentRating)}
onMouseLeave={() => setHover(null)}
onKeyDown={(e) => {
if (e.key === 'Enter' || e.key === ' ') {
setRating(currentRating);
}
}}
tabIndex={0}
role="button"
>
<FaStar />
</span>- Consider adding a visual or text indication of the current rating for better user feedback.
These changes will significantly improve the accessibility and usability of the star rating component.
| initial={{ opacity: 0, y: -10, display:"none", height:0 }} | ||
| animate={{ opacity: 1, y: 0, display: "block", height: "auto" }} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve animation of feedback confirmation message
The current animation approach using display property can lead to layout shifts. Consider using opacity and transform for smoother animations:
<motion.div
initial={{ opacity: 0, y: -10 }}
animate={{ opacity: 1, y: 0 }}
exit={{ opacity: 0, y: -10 }}
transition={{ duration: 0.3 }}
className="mt-4 p-4 bg-green-100 border border-green-400 text-green-700 rounded"
>
Thank you for your feedback!
</motion.div>Also, consider wrapping this component with Framer Motion's AnimatePresence to handle enter/exit animations smoothly:
import { AnimatePresence } from 'framer-motion';
// In your JSX
<AnimatePresence>
{submitted && (
<motion.div
// ... animation props as above
>
Thank you for your feedback!
</motion.div>
)}
</AnimatePresence>This approach provides a smoother animation without causing layout shifts.
it come while deploying it in vercel coz both frontend and backend i will deployed in vercel |
RamakrushnaBiswal
merged commit 7f0a301
into
RamakrushnaBiswal:main
RamakrushnaBiswal
added
enhancement
I added an extra field of rating stars using react-icons
Issue: #61
Thank you, will make more contributions for sure !! :]
Summary by CodeRabbit
New Features
Bug Fixes